-
Notifications
You must be signed in to change notification settings - Fork 103
feat(relay-pattern): Add negation pattern matching #5116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
| fn parse(&mut self) -> Result<(), ErrorKind> { | ||
| if self.advance_if(|c| c == '!') { | ||
| self.push_token(Token::Negated); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Negated Token Incorrectly Affects Wildmatch
The Token::Negated is incorrectly passed into the wildmatch algorithm. This meta-token should be stripped before wildmatch evaluation, as its inclusion causes incorrect matching, especially for patterns like ! which incorrectly match non-empty strings but not empty ones.
Additional Locations (2)
|
@cmanallen have you considered using Generic Inbound Filters for this? We consider the other inbound filters as deprecated and would prefer if new features are implemented through generic filters instead. |
|
|
||
| - Add InstallableBuild and SizeAnalysis data categories. ([#5084](https://github.com/getsentry/relay/pull/5084)) | ||
| - Add dynamic PII derivation to `metastructure`. ([#5107](https://github.com/getsentry/relay/pull/5107)) | ||
| - Add negation pattern matching. ([#5116](https://github.com/getsentry/relay/pull/5116)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong section now.
| /// The pattern is complex and needs to be evaluated using [`wildmatch`]. | ||
| NegatedWildmatch(Tokens), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That doesn't seem correct, a Static literal can also be inverted, we now have this implicit behavior of negated is always a wild match, which also removes a lot of the performance benefits we have by factoring out simple patterns.
You can instead track this property on the Pattern itself and just invert the is_match there.
| if self.advance_if(|c| c == '!') { | ||
| self.push_token(Token::Negated); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should make this behavior configurable via Options and only enable it for releases for now.
We can then just use a TypedPattern with special options for that one glob.
Prefixing
!before a pattern will negate the pattern that follows. This is designed for use within inbound-filters (specifically release filters) where a customer wants to define an allow-list (as opposed to the current deny-list).